feat: expose nucleon number, atom mass, density and is-gas accessors (#119)#124
Open
grzanka wants to merge 3 commits into
Open
feat: expose nucleon number, atom mass, density and is-gas accessors (#119)#124grzanka wants to merge 3 commits into
grzanka wants to merge 3 commits into
Conversation
…119) Promote four previously-internal helpers to the public API so frontends and bindings no longer need their own shims: int dedx_get_nucleon_number(int ion, int *err); float dedx_get_atom_mass(int ion, int *err); float dedx_get_density(int material, int *err); int dedx_is_gas(int target, int *err); The element accessors add the lower-bound check the internal lookups lack (dedx_internal_get_nucleon/_atom_mass only guard the upper bound and would index the table at a negative offset for ion <= 0); out-of-range elements now return -1 with DEDX_ERR_NOT_AN_ELEMENT. dedx_get_density propagates DEDX_ERR_TARGET_NOT_FOUND for unknown materials; dedx_is_gas normalises the internal size_t result to 0/1. Adds tests/test_accessors.c (valid values, out-of-range guards, liquid vs gas) and a short usage demo in examples/dedx_list.c. https://claude.ai/code/session_01XdTFfBgkL344BjVrxARiNc
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
+ Coverage 77.27% 77.66% +0.38%
==========================================
Files 30 31 +1
Lines 3019 3103 +84
Branches 403 409 +6
==========================================
+ Hits 2333 2410 +77
- Misses 597 600 +3
- Partials 89 93 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR exposes four previously-internal material/ion property helpers as part of the public C API (include/dedx.h), to support downstream frontends/bindings (notably the WASM app) without local shims. It adds bounds-checking for element-table access and introduces a new test covering the new accessors, plus a short usage demo in an example.
Changes:
- Add public API declarations for nucleon number, atomic mass, density, and gas/condensed flag accessors.
- Implement the new public accessors in
src/dedx.c, including lower-bound guards for element lookups. - Add a new dedicated test (
tests/test_accessors.c) and updateexamples/dedx_list.cwith a usage snippet.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
include/dedx.h |
Adds public declarations + Doxygen docs for the four new accessors. |
src/dedx.c |
Implements public wrappers and adds element-range guarding for mass/nucleon lookups. |
tests/test_accessors.c |
New test file validating correct values and error/sentinel behavior. |
examples/dedx_list.c |
Adds a short demo showing how to call the new accessors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- examples/dedx_list.c: read each accessor result into a temporary before printing. Passing one &err to several calls inside a single printf left the writes to *err unsequenced (argument evaluation order is unspecified in C). - dedx.h: clarify that dedx_is_gas always sets *err = DEDX_OK; an unknown target is reported as non-gas, not as an error. - test_accessors.c: assert *err == DEDX_OK for the unknown-target is_gas case to lock in the documented contract. https://claude.ai/code/session_01XdTFfBgkL344BjVrxARiNc
Contributor
Author
|
Addressed the review feedback in c70e206:
Generated by Claude Code |
…helpers Route every assertion through three small expect_int/expect_near/expect_true helpers (file-scope failure counter) instead of per-call faili/failmsg/ check_close + inline failmsg() lines. This removes the scattered failure-only statements that showed up as uncovered patch lines; the only lines now uncovered on a passing run are the three shared helper failure branches. Also adds positive cases (oxygen mass, Z=112 upper-boundary acceptance) that exercise more real code paths. test_accessors.c line coverage 87% -> 92% (remaining 8% is the irreducible assert-failed branches); dedx.c accessors remain fully covered with both guard branches exercised. 28/28 ctest pass. https://claude.ai/code/session_01XdTFfBgkL344BjVrxARiNc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #119 · Phase A of the
dedx_extra.cmigration epic #118.What
Promotes four previously-internal helpers to the public API in
dedx.h, so frontends/bindings (notably the WASM web app) no longer need their own shims:The implementations already existed inside libdedx (
dedx_internal_get_nucleon/_atom_massindedx_periodic_table.c,dedx_internal_read_density/_target_is_gasindedx_data_access.c) — they just lacked a public header entry.Why
dedx_web/wasm/dedx_extra.cre-exports exactly these four functions because they had no public declaration. This is the smallest, lowest-risk slice of the migration plan and lets the web side delete its shims. It also unblocks dedx_web #175 (display selected-material density) and #176 (custom density).Notable detail — a latent out-of-bounds guard
dedx_internal_get_nucleon()/dedx_internal_get_atom_mass()only guard the upper bound (id < 113); forion <= 0they would indexdedx_nucl[id-1]/dedx_amu[id-1]at a negative offset. The new public element accessors add the missing lower-bound check and return-1withDEDX_ERR_NOT_AN_ELEMENTfor out-of-range elements.dedx_get_densitypropagatesDEDX_ERR_TARGET_NOT_FOUNDfor unknown materials;dedx_is_gasnormalises the internalsize_tto0/1and reports unknown targets as non-gas (documented).This partially addresses the "bounds-check name/lookup accessors" item in the health-review epic #108.
Tests & docs
tests/test_accessors.c(new, auto-registered by thetest_*.cglob): valid values (H/He/C nucleon numbers and masses, water/air density, liquid-vs-gas), out-of-range guards (0, negative,113,200), and the unknown-material error path. Verified against the real embedded data: Carbon A=12 / 12.0107 u, water 1.0 g/cm³ (non-gas), air 1.205e-3 g/cm³ (gas).examples/dedx_list.c.Verification
ctest: 28/28 pass (27 existing + the new test).clang-format --Werror: clean on all changed files.clang-tidy(the CI config,WarningsAsErrors: '*'): no findings in the added code.Open question for maintainers
Final function names — I followed the names proposed in #119 and the
dedx_get_*convention. Happy to rename (e.g.dedx_get_ion_nucleon_numberto match the web shim, or dropget_for the predicate) if you prefer.Authored via Claude Code as Phase A of the dedx_extra.c → libdedx migration.
Generated by Claude Code